Beacon search endpoint #402
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #402 +/- ##
===========================================
+ Coverage 85.43% 85.52% +0.08%
===========================================
Files 127 127
Lines 4869 4897 +28
Branches 651 657 +6
===========================================
+ Hits 4160 4188 +28
+ Misses 536 535 -1
- Partials 173 174 +1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| search_conf = settings.CONFIG_PUBLIC["search"] | ||
| field_conf = settings.CONFIG_PUBLIC["fields"] | ||
| queryable_fields = { | ||
| f"{f}": field_conf[f] for section in search_conf for f in section["fields"] |
There was a problem hiding this comment.
i think this is a needless format string - it can just be f:
There was a problem hiding this comment.
Oh noes! You want me to pay attention to the code I'm copypasting? 😭
(fixed)
| response = self.client.get('/api/beacon_search?birdwatcher=yes') | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| response_obj = response.json() | ||
| self.assertEqual(response_obj["code"], 400) |
There was a problem hiding this comment.
assert equal to status.HTTP_400_BAD_REQUEST instead (for these 3 tests)
There was a problem hiding this comment.
These are just copies of the old tests, which, yeah, are a bit weird. Changed the response to 400 and updated the tests.
|
|
||
| def get(self, request, *args, **kwargs): | ||
| if not settings.CONFIG_PUBLIC: | ||
| return Response(settings.NO_PUBLIC_DATA_AVAILABLE) |
There was a problem hiding this comment.
should this not be a 404 instead of a 200? fine to leave as-is for consistency with bento public responses but generally i think in API design it's good to put as much meaning in the status code as possible
There was a problem hiding this comment.
Yeah, this was a straight copypaste of the bento_public code, and it struck me as odd also. I won't break the old code, but I'll change the beacon responses to 400 / bad request. (even for missing config, arguably still 400 rather than 404)
There was a problem hiding this comment.
hmm i think of 400s more meaning something is wrong with the request... if the client request is good, I still think this should be a 404 (or maybe a 500 if we treat it as a configuration error)
There was a problem hiding this comment.
Ah, okay, I actually re-read the documentation for 404 and this sounds reasonable ("missing resource"). 204 (No content) is also possible but probably a little obscure. I'll change it to 404.
davidlougheed
changed the title
Create a new katsu endpoint
/api/beacon_search, an uncensored equivalent to the bento_public search endpoint/api/public. Returns a full list of matching individual ids rather than censored counts, so that beacon can compute the intersection of results with other searches (in particular for variants). Beacon instead of katsu will apply censorship in this case.This allows beacon to use katsu's config for search, which gets us: